Skip to content

Optimize single-replace edit path#39

Merged
Sewer56 merged 1 commit into
mainfrom
optimize-edit-single-replace
Feb 27, 2026
Merged

Optimize single-replace edit path#39
Sewer56 merged 1 commit into
mainfrom
optimize-edit-single-replace

Conversation

@Sewer56
Copy link
Copy Markdown
Member

@Sewer56 Sewer56 commented Feb 27, 2026

Rationale

  • Optimize performance by avoiding full occurrence counting in single-replace mode.

Expected changes

  • Single-replace exits after detecting a second match.
  • Ambiguous-match errors report "multiple times" instead of an exact count.
  • replace_all behavior and exact replacement-count reporting remain unchanged.

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 27, 2026

Codecov Report

❌ Patch coverage is 81.81818% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.10%. Comparing base (9d0809f) to head (0aed2a6).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/llm-coding-tools-core/src/tools/edit.rs 80.00% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #39      +/-   ##
==========================================
- Coverage   75.25%   75.10%   -0.16%     
==========================================
  Files          67       67              
  Lines        1964     1976      +12     
==========================================
+ Hits         1478     1484       +6     
- Misses        486      492       +6     
Flag Coverage Δ
unittests 75.10% <81.81%> (-0.16%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/llm-coding-tools-serdesai/src/absolute/edit.rs 15.78% <ø> (ø)
src/llm-coding-tools-serdesai/src/common/edit.rs 66.66% <100.00%> (+3.03%) ⬆️
src/llm-coding-tools-core/src/tools/edit.rs 73.33% <80.00%> (-0.36%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Feb 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e4cac87 and 0aed2a6.

📒 Files selected for processing (3)
  • src/llm-coding-tools-core/src/tools/edit.rs
  • src/llm-coding-tools-serdesai/src/absolute/edit.rs
  • src/llm-coding-tools-serdesai/src/common/edit.rs
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Build and Test (Async/Tokio) (macos-latest, aarch64-apple-darwin, false)
  • GitHub Check: Build and Test (Blocking) (windows-latest, x86_64-pc-windows-msvc, false)
  • GitHub Check: Build and Test (Blocking) (macos-latest, aarch64-apple-darwin, false)
  • GitHub Check: Build and Test (Async/Tokio) (windows-latest, x86_64-pc-windows-msvc, false)
  • GitHub Check: Build and Test (Async/Tokio) (ubuntu-latest, x86_64-unknown-linux-gnu, false)
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.rs

📄 CodeRabbit inference engine (src/AGENTS.md)

src/**/*.rs: Preallocate collections when size is known or estimable using String::with_capacity(estimated_len), Vec::with_capacity(count), or BufReader::with_capacity(size, reader)
Prefer &str / &[T] returns over owned types when lifetime allows
Use Cow<'_, str> for conditional ownership (e.g., String::from_utf8_lossy)
Use &'static str for compile-time constant strings
Reuse buffers by calling .clear() and reusing Vec/String instead of reallocating
Use const generics for compile-time branching (e.g., <const LINE_NUMBERS: bool>)
Use #[inline] on small, hot-path functions
Prefer core over std where possible (e.g., core::mem over std::mem)
Use memchr crate for fast byte searching over manual iteration
Keep modules under 500 lines (excluding tests); split if larger
Place use statements inside functions only for #[cfg] conditional compilation
Document public items with /// and add examples in docs where helpful
Use //! for module-level documentation
Use [TypeName] rustdoc links instead of backticks in documentation

Files:

  • src/llm-coding-tools-serdesai/src/common/edit.rs
  • src/llm-coding-tools-serdesai/src/absolute/edit.rs
  • src/llm-coding-tools-core/src/tools/edit.rs
🧠 Learnings (2)
📚 Learning: 2026-02-07T22:53:26.067Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.067Z
Learning: Organize code with `llm-coding-tools-serdesai/` containing `src/absolute/`, `src/allowed/`, `src/schema.rs`, and `src/convert.rs` for serdesAI framework implementations

Applied to files:

  • src/llm-coding-tools-serdesai/src/common/edit.rs
📚 Learning: 2026-02-07T22:53:26.067Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.067Z
Learning: Organize code with `llm-coding-tools-core/` as framework-agnostic core library containing `src/tools/`, `src/path/`, `src/error.rs`, `src/output.rs`, and `src/util.rs`

Applied to files:

  • src/llm-coding-tools-core/src/tools/edit.rs
🧬 Code graph analysis (1)
src/llm-coding-tools-core/src/tools/edit.rs (2)
src/llm-coding-tools-core/src/fs/blocking_impl.rs (1)
  • write (12-14)
src/llm-coding-tools-core/src/fs/tokio_impl.rs (1)
  • write (12-14)
🔇 Additional comments (5)
src/llm-coding-tools-core/src/tools/edit.rs (3)

24-30: LGTM: Clean enum variant simplification.

The documentation clearly explains the rationale for removing the count from AmbiguousMatch - it enables the early-exit optimization in single-replace mode.


60-89: Well-optimized single-replace path with proper preallocation.

The implementation correctly:

  • Exits early after detecting a second match (lines 76-78)
  • Preallocates the result string with exact capacity (line 83-84)
  • Builds the result from slices without rescanning (lines 85-87)

The capacity calculation content.len() - old_string.len() + new_string.len() is precise.

Note: The coding guidelines mention using memchr crate for fast byte searching, but that applies to single-byte searches. For substring matching like this, match_indices is the appropriate choice since it handles multi-byte patterns correctly.


93-96: LGTM: Success message now uses actual replacement count.

The format string correctly reports the count from either code path.

src/llm-coding-tools-serdesai/src/absolute/edit.rs (1)

180-180: LGTM: Test assertion updated to match new error message.

The test correctly validates the new static "multiple times" message instead of the previous dynamic count.

src/llm-coding-tools-serdesai/src/common/edit.rs (1)

21-26: LGTM: Error conversion updated for unit variant.

The match arm correctly handles the simplified AmbiguousMatch unit variant and provides a consistent error message.


Walkthrough

The EditError::AmbiguousMatch enum variant was changed from AmbiguousMatch(usize) to a unit variant AmbiguousMatch. edit_file now branches explicitly: in replace_all it computes the exact replacement count and returns NotFound if zero; in single-replace it scans for the first occurrence, returns NotFound if none, returns AmbiguousMatch if a second occurrence exists, and constructs the edited content without rescanning, reporting one replacement. Tests and error serialization were updated to use a static "multiple times" message.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Optimize single-replace edit path' accurately summarizes the main objective of the PR—improving performance in the single-replace mode by avoiding full occurrence counting.
Description check ✅ Passed The description covers the rationale (performance optimization) and expected changes (single-replace behavior, error message updates, and unchanged replace_all), directly addressing the PR objectives.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch optimize-edit-single-replace

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Rationale: single replace only needs to detect whether matches are 0, 1, or multiple.

Expected changes: single-replace exits after the second match instead of counting all matches, ambiguous-match errors report "multiple times", and replace_all behavior stays the same.
@Sewer56 Sewer56 force-pushed the optimize-edit-single-replace branch from e4cac87 to 0aed2a6 Compare February 27, 2026 21:12
@Sewer56 Sewer56 merged commit 5c8d961 into main Feb 27, 2026
8 checks passed
@Sewer56 Sewer56 deleted the optimize-edit-single-replace branch February 27, 2026 21:16
Sewer56 added a commit that referenced this pull request Mar 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant